Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't override intentionally set project names. #147

Merged
merged 1 commit into from
Jan 19, 2017

Conversation

relistan
Copy link
Collaborator

This fix is grabbed from #128 thanks to @MarkBorcherding. It will change the behavior of Centurion to fix a bug, but may have the unintended consequence of changing the names of services that are currently running and have been getting the name of the file rather than the :name they tried to set. I know that this might break some deployments at New Relic and will need to be investigated before this can be merged. The effect would be duplicate services running in the worst case, one new version, one old. For services that bind to ports the risk of this is lower since they will collide. But it would need manual intervention to fix. We need to investigate all the service configs to see if this is an issue.

cc @benders @dselans

@rngtng
Copy link

rngtng commented Mar 31, 2016

What's status on this? In case this needs more time, can we at least remove it from readme? It took me a while the mistake wasn't on my side :(

@tristanholl
Copy link

👍

@relistan
Copy link
Collaborator Author

This should really be merged @dselans but you guys will need to validate that it doesn't change project names anywhere. Validating that is a matter of checking the filename vs the service name.

@relistan
Copy link
Collaborator Author

relistan commented Jun 9, 2016

@actaeon help getting this tested so a merge doesn't break a lot of projects would be appreciated.

@actaeon
Copy link
Contributor

actaeon commented Jun 9, 2016

@relistan offsite this week :) , will look at it Monday.

@relistan
Copy link
Collaborator Author

@intjonathan @actaeon @spkane want to merge or close this. Can you guys check it out, or help get someone else to?

@intjonathan
Copy link
Contributor

I'm going to do a 1.8.9 with the timeout fixes first, then come back to this as a 1.8.10.

@intjonathan intjonathan merged commit 816ec5b into master Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants